-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: export distribution container build artifacts #2186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c151c80
to
5fe4f8a
Compare
ac4cacd
to
0320648
Compare
Add a new --export-dir flag to the `llama stack build` command that allows users to export container build artifacts to a specified directory instead of building the container directly. This feature is useful for: - Building containers in different environments - Sharing build configurations - Customizing the build process The exported tarball includes: - Containerfile (Dockerfile) - Run configuration file (if building from config) - External provider files (if specified) - Build script for assistance The tarball is named with a timestamp for uniqueness: <distro-name>_<timestamp>.tar.gz Documentation has been updated in building_distro.md to reflect this new functionality as well as integration tests. Signed-off-by: Sébastien Han <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Here's a first round of review around the UI, and I shared some arguments why having a dedicated llama stack export
would possibly make sense.
@@ -71,6 +71,8 @@ options: | |||
found. (default: None) | |||
--print-deps-only Print the dependencies for the stack only, without building the stack (default: False) | |||
--run Run the stack after building using the same image type, name, and other applicable arguments (default: False) | |||
--export-dir EXPORT_DIR | |||
Export the build artifacts to a specified directory instead of building the container. This will create a tarball containing the Dockerfile and all necessary files to build the container. (default: None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe break up the overlong line into two and align, like for the --config
option. However this is not consistent, maybe we should agree on a max. line length for help texts (like 80 or 120)
You can also export the build artifacts to a specified directory instead of building the container directly. This is useful when you want to: | ||
- Build the container in a different environment | ||
- Share the build configuration with others | ||
- Customize the build process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Customize the build process | |
- Customize the build process | |
- Integrate into CI/CD pipelines |
``` | ||
|
||
This will create a tarball in the specified directory containing: | ||
- The Dockerfile (named Containerfile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The Dockerfile (named Containerfile) | |
- The Containerfile (also known as Dockerfile) |
- The run configuration file (if building from a config) | ||
- Any external provider files (if specified in the config) | ||
|
||
The tarball will be named with a timestamp to ensure uniqueness, for example: `<distro-name>_<timestamp>.tar.gz` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether we should not directly export the files as-is, and let the user do the archiving (if needed).
If using an archive instead like proposed, I would rethink the UI, and instead of adding an --export-dir
use an --export-archive <archive>
with an optional argument for the the name of the archive to generate. If not given, use the proposed default name. Always create it in the local directory (except when you add a directory path to the argument)
We could also do it all at once, with the danger of including too much magic:
- Just use
--export
as the option name - Allow an optional argument
<archive>
- If not
<archive>
given, then create a<distron-name>_<timestamp>
.tar.gz in the current directory - If
<archive>
is given:- If archive is a directory (either existing or name ends with
/
), then store the files directly in this directory (create it or throw an error if the dir does not exist) - If archive is a file-name, use that as an archive name and create is (.zip or .tar.gz, depending on the name)
- If archive is a directory (either existing or name ends with
Regardless what we chose, I would not unconditionally add a timestamp, as this might complicate CI handling when both the lls build
call and a custom build strategy would be part of the same workflow task (the later step would need to identify that archive/dir). At least make it optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the behaviour change to llama stack build
is to large (e.g. nothing is really build), then an alternative would be to allow a llama stack export
command, where we then are free to add more options (like --output-dir
, --archive
, ....) to fine tune the behaviors.
I think this makes even more sense in the context of the other options of llama stack build
like --run
, which makes no sense if --export
is used.
What does this PR do?
Add a new --export-dir flag to the
llama stack build
command that allows users to export container build artifacts to a specified directory instead of building the container directly. This feature is useful for:The exported tarball includes:
The tarball is named with a timestamp for uniqueness: _.tar.gz
Documentation has been updated in building_distro.md to reflect this new functionality as well as integration tests.
Test Plan